Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-42846][SQL] Remove error condition _LEGACY_ERROR_TEMP_2011 #48086

Closed

Conversation

ivanjevtic-db
Copy link
Contributor

@ivanjevtic-db ivanjevtic-db commented Sep 12, 2024

What changes were proposed in this pull request?

Removed error condition _LEGACY_ERROR_TEMP_2011, removed dataTypeUnexpectedError, typeUnsupportedError, and replaced them with SparkException.internalError.

Why are the changes needed?

It is impossible to trigger the error from user space.
Here I changed dataTypeUnexpectedError to internalError, since typeSoFar argument will always be either:

  • NullType if this is the first row or
  • Some type which is returned by the inferField function(which is a valid type).

Here I changed typeUnsupportedError to internalError, since:

  • in this function call, the exception will be caught and
  • in this function call, a valid desiredType will always be passed.

Here and here I changed dataTypeUnexpectedError to internalError, since there is a type signature here which prevents child.dataType from being unexpected type.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Build passing.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the SQL label Sep 12, 2024
@ivanjevtic-db ivanjevtic-db marked this pull request as draft September 12, 2024 10:13
@ivanjevtic-db ivanjevtic-db marked this pull request as ready for review September 12, 2024 10:13
@ivanjevtic-db ivanjevtic-db changed the title [SPARK-42846] Remove error class _LEGACY_ERROR_TEMP_2011 [SPARK-42846][SQL] Remove error class _LEGACY_ERROR_TEMP_2011 Sep 12, 2024
Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update PR's title and description, and replace error class by error condition. See #48027

@ivanjevtic-db ivanjevtic-db changed the title [SPARK-42846][SQL] Remove error class _LEGACY_ERROR_TEMP_2011 [SPARK-42846][SQL] Remove error condition _LEGACY_ERROR_TEMP_2011 Sep 12, 2024
@MaxGekk
Copy link
Member

MaxGekk commented Sep 12, 2024

@ivanjevtic-db Do you have an account at OSS JIRA? If so, please, leave some comment in https://issues.apache.org/jira/browse/SPARK-42846 otherwise submit request:
Screenshot 2024-09-12 at 14 39 14

@ivanjevtic-db
Copy link
Contributor Author

I requested, will add comment when I get access.

@MaxGekk
Copy link
Member

MaxGekk commented Sep 12, 2024

+1, LGTM. Merging to master.
Thank you, @ivanjevtic-db.

@MaxGekk MaxGekk closed this in bc54eac Sep 12, 2024
@MaxGekk
Copy link
Member

MaxGekk commented Sep 12, 2024

@ivanjevtic-db Congratulations with your first contribution to Apache Spark!

attilapiros pushed a commit to attilapiros/spark that referenced this pull request Oct 4, 2024
### What changes were proposed in this pull request?
Removed error condition **_LEGACY_ERROR_TEMP_2011**, removed **dataTypeUnexpectedError**, **typeUnsupportedError**, and replaced them with **SparkException.internalError**.

### Why are the changes needed?
It is impossible to trigger the error from user space.
[Here](https://github.com/apache/spark/compare/master...ivanjevtic-db:spark:remove-legacy-error-temp-2011?expand=1#diff-688ac8011f7fb514154ff57cfb1278b15aec481d68c1a499c90f8a330d3a42a1L141) I changed dataTypeUnexpectedError to internalError, since _typeSoFar_ argument will always be either:
- NullType if this is the first row or
- Some type which is returned by the [_inferField_](https://github.com/apache/spark/compare/master...ivanjevtic-db:spark:remove-legacy-error-temp-2011?expand=1#diff-688ac8011f7fb514154ff57cfb1278b15aec481d68c1a499c90f8a330d3a42a1L125) function(which is a valid type).

[Here](https://github.com/apache/spark/compare/master...ivanjevtic-db:spark:remove-legacy-error-temp-2011?expand=1#diff-e9a88a888c1543c718c24f25036307cb32348ca3f618a8fa19240bdc3c0ffaf4L553) I changed typeUnsupportedError to internalError, since:
- in [this](https://github.com/apache/spark/compare/master...ivanjevtic-db:spark:remove-legacy-error-temp-2011?expand=1#diff-e9a88a888c1543c718c24f25036307cb32348ca3f618a8fa19240bdc3c0ffaf4L204) function call, the exception will be caught and
- in [this](https://github.com/apache/spark/compare/master...ivanjevtic-db:spark:remove-legacy-error-temp-2011?expand=1#diff-e9a88a888c1543c718c24f25036307cb32348ca3f618a8fa19240bdc3c0ffaf4L367) function call, a valid _desiredType_ will always be passed.

[Here](https://github.com/apache/spark/compare/master...ivanjevtic-db:spark:remove-legacy-error-temp-2011?expand=1#diff-d1fa4a2cbd66cff7d7d8a90d7ac70457a31e906cebb7d43a46a6036507fb4e7bL192) and [here](https://github.com/apache/spark/compare/master...ivanjevtic-db:spark:remove-legacy-error-temp-2011?expand=1#diff-d1fa4a2cbd66cff7d7d8a90d7ac70457a31e906cebb7d43a46a6036507fb4e7bL217) I changed dataTypeUnexpectedError to internalError, since there is a type signature [here](https://github.com/apache/spark/compare/master...ivanjevtic-db:spark:remove-legacy-error-temp-2011?expand=1#diff-d1fa4a2cbd66cff7d7d8a90d7ac70457a31e906cebb7d43a46a6036507fb4e7bL104) which prevents _child.dataType_ from being unexpected type.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Build passing.

### Was this patch authored or co-authored using generative AI tooling?
No.

Closes apache#48086 from ivanjevtic-db/remove-legacy-error-temp-2011.

Authored-by: ivanjevtic-db <ivan.jevtic@databricks.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants